-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
[build] Ensure all valid ruby tests are running on RBE #15718
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||
CI Feedback 🧐(Feedback updated until commit 67d372e)A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
.bazelrc.remote
Outdated
| build:rbe --experimental_inmemory_jdeps_files | ||
| build:rbe --remote_timeout=3600 | ||
| build:rbe --spawn_strategy=remote,local | ||
| build:rbe --spawn_strategy=remote |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
turns out this doesn't fix the problem I thought it would, so I'll revert it.
For some reason, when I'm running --config=rbe anything with tag exclusive-with-local still runs in series instead of parallel. I'm not sure why. @shs96c do you know why this may be?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(it makes a huge difference time-wise when I remove the exclusive-with-local tags and running on rbe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well known issue bazelbuild/bazel#17834
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, so this is probably why we were previously skipping them on rbe to avoid the performance hit
… parallel" This reverts commit ca2b267.
| rb_integration_test( | ||
| name = "service", | ||
| srcs = ["service_spec.rb"], | ||
| browsers = ["chrome"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to add beta chrome here as part of the browsers
|
Moving to draft. Might be replaced with #16484 |
User description
🔗 Related Issues
We've seen a few issues in other bindings for problems hidden by tests in the skipped-test file
💥 What does this PR do?
bidi-remoteruby test targetsbidi-supportedanddev-tools-supportedfor specific browsers.🔧 Implementation Notes
💡 Additional Considerations
🔄 Types of changes
PR Type
Enhancement, Bug fix
Description
Refactored Ruby test build logic for better variant handling
bidi-only,remote-only, andno-gridtagsbidi_supported,devtools_supported)Fixed WebSocket connection error handling in Ruby bindings
Thread.stopwithThread.exitfor cleaner thread terminationCleaned up and reorganized Bazel Ruby test targets
.skipped-testsBUILD.bazelfiles for browser-specific test execution and taggingMinor test and helper improvements
Changes walkthrough 📝
10 files
Refactor test target generation and browser capability taggingEnhance wait helper to accept extra optionsAdd/modify test targets for bidi and network; update tagsAdd bidi-only tag to all BiDi test targetsSeparate service test with no-grid/skip-rbe tagsSeparate service test with no-grid/skip-rbe tagsSeparate service test with no-grid/skip-rbe tagsAdd new service test target for IE with no-grid tagAdd grid-only tag to remote test targetsSeparate service test with no-grid tag for Safari5 files
Improve WebSocket thread error handling and cleanupFix test exclusion key for FirefoxUpdate test exclusivity to only Chrome/EdgeImprove waiting logic and test exclusion for downloadsFix file upload parameter handling in Rack server2 files
Remove local spawn strategy from RBE configRemove Ruby integration test targets from skip list1 files